Fix/naws no horizontal scroll - #10
Merged
Merged
Conversation
The font is fitted from a measured 200-char glyph run so exactly MinColumns (78) columns span the content box. But the min-width track that holds those columns used the CSS `ch` unit (`calc(var(--sc-cols) * 1ch)`), whose single- glyph advance rounds ~2px wider than the averaged run. So `78ch` exceeded the box by a couple px and .sc-output-area's overflow-x:auto showed a permanent tiny horizontal scrollbar — you had to center the scroll to see all 78, and long lines scrolled instead of wrapping at the column edge. measureGrid now publishes the track width in pixels (--sc-cols-width), measured from a real run of `cols` glyphs at the fitted size — the same basis the font was fitted to. .sc-output min-width uses that px value instead of 1ch. In the fitting case the track stays inside the box (no scroll; lines wrap at column 78); it only exceeds the box and scrolls when the screen genuinely can't fit the columns at the minimum font, which is the intended fallback. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HHCRc5CJ6595iMzEgYYdQp
The earlier ch→px change was a no-op: verified in-browser that for JetBrains Mono `78ch` and the measured 78-column run are sub-pixel identical, so the column track was never the ~2px too wide I'd assumed. The horizontal scroll came from the page *allowing* it — `.sc-output` had a forced `min-width` column track and `.sc-output-area` had `overflow-x: auto`, so any excess width (sub-pixel rounding, or Android WebView font-boosting inflating text past the size the grid was fitted to) became a left/right scrollbar you had to center to read all 78 columns. Fix, matching the intent (wrap, never scroll sideways): - Remove the `min-width` track from `.sc-output` — nothing forces the block wider than its box, so lines wrap at the fitted column width. - `.sc-output-area` overflow-x: auto → hidden — the page can no longer scroll horizontally; any residual excess is clipped, not scrolled. - Add `text-size-adjust: 100%` on html/body to stop the Android WebView from auto-inflating text (the device-only reason real lines exceeded the fitted grid), and let `.sc-link` buttons wrap so a long command link can't overflow. - Revert the unused `--sc-cols-width` JS. Verified in Chrome across 360/393/412px widths with a long link + simulated boosting: no horizontal scrollbar; content wraps. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HHCRc5CJ6595iMzEgYYdQp
Follow-up to the horizontal-scroll fix, addressing two things reported on device: the output was inset ~5 columns short of the screen width (78-col lines wrapped after 73), and the bottom nav was taller than needed. - .sc-output-area horizontal padding 1rem → 4px: a wide inset wasted ~5 columns at the fitted font size. When the box is small enough that the font floors at 6px, reclaimed raw width is the only way to fit the advertised columns, so this directly recovers them. Vertical padding unchanged. - observeResize now re-fires the fit callback on document.fonts.ready and after a couple of animation frames. The first measurement can run before the native WindowInsets bridge has applied the real safe-area padding (which shrinks the box); re-measuring once the layout settles re-fits the grid to the true width instead of leaving lines wrapping a few chars short. - .sc-nav-link: padding 8/7 → 4/3, icon 21 → 19px, gap 3 → 2px — a shorter Session/World/etc bar. Web builds clean. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HHCRc5CJ6595iMzEgYYdQp
There was a problem hiding this comment.
Pull request overview
This PR targets unwanted horizontal scrolling in the session output view by ensuring the terminal grid is measured against the final layout (after fonts/insets settle) and by changing the CSS layout model to prefer wrapping/clipping over horizontal overflow.
Changes:
- Add delayed “re-fire” resize notifications (fonts ready + a couple animation frames) to stabilize initial NAWS/font/grid sizing.
- Disable Android WebView text inflation and remove CSS min-width/overflow-x behaviors that created horizontal scrollbars.
- Tighten output padding and ensure clickable command links can wrap/break to avoid forcing overflow.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/SharpClient.UI/wwwroot/sc-interop.js | Re-fires resize after async layout settling to improve initial grid/NAWS accuracy. |
| src/SharpClient.UI/wwwroot/app.css | Removes horizontal scrolling mechanics and adjusts wrapping/padding to prevent overflow. |
Comments suppressed due to low confidence (1)
src/SharpClient.UI/wwwroot/app.css:86
- CSS now prevents horizontal overflow (removes the --sc-cols min-width track and hides overflow-x), but measureGrid() still reports
cols = targetColseven when it clamps at the minimum font size and can no longer fittargetColsinto the content box. On narrow screens (or with Min columns set high, up to 120), this will send an inflated NAWS width to the server, so server-wrapped lines won’t match the client’s visible wrap.
font-size: var(--out-fs);
}
.sc-line {
font-family: var(--mono);
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+38
to
+51
| const refire = () => { | ||
| const cs2 = getComputedStyle(element); | ||
| const px = parseFloat(cs2.paddingLeft || '0') + parseFloat(cs2.paddingRight || '0'); | ||
| const py = parseFloat(cs2.paddingTop || '0') + parseFloat(cs2.paddingBottom || '0'); | ||
| const w = Math.floor(element.clientWidth - px); | ||
| const h = Math.floor(element.clientHeight - py); | ||
| if (w > 0) { | ||
| dotNetRef.invokeMethodAsync('OnResized', w, h); | ||
| } | ||
| }; | ||
| if (document.fonts && document.fonts.ready) { | ||
| document.fonts.ready.then(refire); | ||
| } | ||
| requestAnimationFrame(() => requestAnimationFrame(refire)); |
Comment on lines
+34
to
+38
| /* Stop the Android WebView from auto-inflating ("boosting") text in block containers based on the | ||
| system font-size setting — it renders lines wider than the size measureGrid fitted the column | ||
| grid to, breaking the NAWS column math and forcing horizontal overflow. Keep text at the CSS px. */ | ||
| -webkit-text-size-adjust: 100%; | ||
| text-size-adjust: 100%; |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.